-
Notifications
You must be signed in to change notification settings - Fork 0
Bump gridsuite dependencies to 32 #141
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Seddik Yengui <[email protected]>
Signed-off-by: Seddik Yengui <[email protected]>
Signed-off-by: Seddik Yengui <[email protected]>
mockMvc.perform(put("/" + VERSION + "/results/" + RESULT_UUID + "/stop" | ||
+ "?receiver=me")) | ||
mockMvc.perform(put("/" + VERSION + "/results/" + RESULT_UUID + "/stop" + "?receiver=me") | ||
.header(HEADER_USER_ID, "testUserId")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it seems here we manage to get the stopped notification and not the cancelfailed notification. Why ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think because we running in different thread and we wait
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// wait for security analysis to actually run before trying to stop it
countDownLatch.await();
And because in this service we have a SecurityAnalysisProviderMock
which make:
// creating a long completable future which is here to be canceled
return new CompletableFuture<SecurityAnalysisReport>().completeOnTimeout(REPORT, 3, TimeUnit.SECONDS);
then we have the time to cancel the computation and obtain a clean sa.stopped
Both of those code parts seems absent of other computation services (LF, SE,Dy, CC etc...)
Should add it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should add another test to test the cancelfailed notif no ?
Signed-off-by: Seddik Yengui <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review OK
|
No description provided.